The same would probably also need to be done for everything in .ui.sheets, i.e. all bottom sheets since they're dialogs as well.

Please save all the keys used for the bundles inside the IntentData object as a constant so that we can avoid typos in the key and keep the code cleaner.

Please save all the keys used for the bundles inside the IntentData object as a constant so that we can avoid typos in the key and keep the code cleaner.

Ok. Should I commit from here directly by clicking on 'Commit Suggestion' button or by pushing the changes to the branch?

It would be better if you do the suggested changes on your PC and push it via git as usual.

It would be better if you do the suggested changes on your PC and push it via git as usual.

Ok.

Thanks for your work on that and applying the suggestions, looks fine to merge from my side when we can get the FragmentResultListener to work.

If you want to have a look at the bottom sheets as well: we sometimes pass the player there, the easiest thing probably would be to save the player in the PlayerViewModel and use it from there.

If you want to have a look at the bottom sheets as well: we sometimes pass the player there, the easiest thing probably would be to save the player in the PlayerViewModel and use it from there.

Ok.

A couple of things I want to tell:

1. I have removed the arguments from the remaining dialog's constrcutors except the dialogs involving PlaylistOptionsBottomSheet due to some problem with FragmentResultListener and the ColorPickerDialog (2nd point). I'll push the changes later.

2. In ColorPickerDialog there an argument for Context. I am not sure how to pass it through Bundle. I will think of its workaround later.

3. There is conflict in app/src/main/java/com/github/libretube/ui/preferences/MainSettings.kt due to the recent push in master. I am not sure what to do for it. Resolve Conflicts button is appearing for me in this PR.

1. Sounds good 👍
2. We can't pass the context like that, however bottom sheets should have its own context after onCreate was called. So just make everything that is initialized with the context lateinit var and create it in the onCreate function.
3. No worries about that, I can take care of it when merging.

Why status checks aren't running ?

Probably due to the merge conflicts.
I don't have my PC with me currently, but in theory you can just click the resolve conflicts button and remove the two or three lines indicating that there's a conflict (it should be fine to just keep all the code as it is there).

1. Sounds good 👍
2. We can't pass the context like that, however bottom sheets should have its own context after onCreate was called. So just make everything that is initialized with the context lateinit var and create it in the onCreate function.
3. No worries about that, I can take care of it when merging.

2. ColorPickerDialog is working as expected without supplying the context. It only uses to context for showing toast messages.
3. Could you please check the MainSettings.kt that I have pushed?

Everything is done. If there aren't any things need to be corrected from your side, I will push the remaining Dialogs.

MainSettings.kt looks good to me 👍

Is there something wrong with IntentData.kt, code quality check is failing.

There's a package called ktlint for Linux, with that one installed you can run ktlint --code-style=android-studio -F to automatically format the code.

There's a package called ktlint for Linux, with that one installed you can run ktlint --code-style=android-studio -F to automatically format the code.

Ktlint doesn't show anything wrong in IntentData for me. I use windows.

There's a package called ktlint for Linux, with that one installed you can run ktlint --code-style=android-studio -F to automatically format the code.

I have tried placing the IntentData in a new Android Project with ktlint setup and it. After doing the ktlint format command the code in IntentData remains same and doesn't give any errors.

It asks us to use variable names like PLAYLIST_DESCRIPTION instead of playlistDescription, but we can safely ignore this.

If everything works from your perspective, we can unmark it as draft. However, I'd like to test this a bit myself (and others as well) first before merging to avoid any new issues by the changes.

Sure. Just send me a reply for any changes or to unmark from draft after you have tested.

1. When renaming a playlist or changing its description it's not visible in the UI immediately as it used to, the playlist has to be reloaded which we should fix before merging.
2. When changing the theme mode, all crashes while a dialog is opened seem to be gone (which is great). However, we still need to do the same thing for all bottom sheets to fix the issue, but that can be done by someone else in a new PR.

If 1. is fixed, it's okay to be merged from my POV.

1. When renaming a playlist or changing its description it's not visible in the UI immediately as it used to, the playlist has to be reloaded which we should fix before merging.
2. When changing the theme mode, all crashes while a dialog is opened seem to be gone (which is great). However, we still need to do the same thing for all bottom sheets to fix the issue, but that can be done by someone else in a new PR.

If 1. is fixed, it's okay to be merged from my POV.

1. Please recheck. It is updated immediately. Your renaming code isn't changed by me. Only thing that is changed is where the code is called.

No, that's clearly not working as it does before the changes.

We can consider passing the callback functions differently instead of using the FragmentResultListener:

val renamePlaylistDialog = RenamePlaylistDialog()
renamePlaylistDialog.onRename = {
    // insert the callback function content here
}
renamePlaylistDialog.show(fragmentManager, "foo")

where onRename is a public variable of type () -> Unit.

We can consider passing the callback functions differently instead of using the FragmentResultListener:

val renamePlaylistDialog = RenamePlaylistDialog()
renamePlaylistDialog.onRename = {
    // insert the callback function content here
}
renamePlaylistDialog.show(fragmentManager, "foo")

where onRename is a public variable of type () -> Unit.

Ok.

We can consider passing the callback functions differently instead of using the FragmentResultListener:

val renamePlaylistDialog = RenamePlaylistDialog()
renamePlaylistDialog.onRename = {
    // insert the callback function content here
}
renamePlaylistDialog.show(fragmentManager, "foo")

where onRename is a public variable of type () -> Unit.

Done.

1. When renaming a playlist or changing its description it's not visible in the UI immediately as it used to, the playlist has to be reloaded which we should fix before merging.
2. When changing the theme mode, all crashes while a dialog is opened seem to be gone (which is great). However, we still need to do the same thing for all bottom sheets to fix the issue, but that can be done by someone else in a new PR.

If 1. is fixed, it's okay to be merged from my POV.

Any other changes ?

Are you interested in looking into the bottom sheets too, or shall I do that?

Are you interested in looking into the bottom sheets too, or shall I do that?

I would but I am kinda busy nowadays due to college. Thanks.

We're not in a hurry, you can do it whenever you want :)

We're not in a hurry, you can do it whenever you want :)

Ok. Sounds good.

For your information, I've already done the changes in some separate PRs now.

Nevertheless, feel free to work on any other issue whenever you want to :)

For your information, I've already done the changes in some separate PRs now.

Nevertheless, feel free to work on any other issue whenever you want to :)

Ok 👍